feat: include reason label to `kube_deployment_status_condition` #2719

Rishab87 · 2025-08-04T16:34:52Z

What this PR does / why we need it:
Building upon the work of #2146, I've addressed the reviews and created this.

This PR adds reason label to kube_deployment_status_condition metrics.

It's necessary to distinguish between Progressing Deployment and Complete Deployment .

How does this change affect the cardinality of KSM: (increases, decreases or does not change cardinality)
no changes in cardinality

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #1991

mrueg · 2025-08-06T08:24:52Z

internal/store/deployment.go

-						metric.LabelValues = append([]string{string(c.Type)}, metric.LabelValues...)
+						reason := c.Reason
+						if _, ok := allowedDeploymentReasons[reason]; !ok {
+							reason = ""


should this be empty? or should this be "unknown"

CC: @rexagod

I went with an empty string for cleaner queries, but I'm open to changing it if you want.

it depends, is the reason ever empty? then we could use unknown to differ between an empty reason and something we don't recognize.

if the reason field is never empty, this approach is fine.

yeah right, seems like reason field can be empty, I've changed it to unknown, thanks for pointing out!

dgrisonnet · 2025-08-07T16:51:47Z

/assign @mrueg
/triage accepted

Rishab87 · 2025-08-11T12:06:41Z

@mrueg I've made the changes, can you have a look again?

mrueg · 2025-08-11T13:51:16Z

internal/store/deployment.go

-						metric.LabelKeys = []string{"condition", "status"}
-						metric.LabelValues = append([]string{string(c.Type)}, metric.LabelValues...)
+						reason := c.Reason
+						if _, ok := allowedDeploymentReasons[reason]; !ok {


if it's empty, will it still be unknown?
Can you create a test with the reason field being empty as well as with one test with a non allowed deployment reason?

yeah that was the case earlier, fixed it if its empty then it will remain empty and will show unknown if reason is not in the allowed list, also added the tests for both cases.

Rishab87 · 2025-08-11T14:48:24Z

@mrueg made changes, can you have another look?

mrueg · 2025-08-11T15:09:36Z

internal/store/deployment.go

@@ -183,8 +183,10 @@ func deploymentMetricFamilies(allowAnnotationsList, allowLabelsList []string) []
 						metric := m

 						reason := c.Reason
-						if _, ok := allowedDeploymentReasons[reason]; !ok {
-							reason = "unknown"
+						if reason != "" {


Can we add "" to the allowedDeploymentReasons instead?

yeah, added it

Rishab87 · 2025-08-12T13:43:40Z

@mrueg I've addressed the reviews, can you look again?

mrueg · 2025-08-12T14:20:15Z

internal/store/deployment.go

+var (
+	allowedDeploymentReasons = map[string]struct{}{
+		"MinimumReplicasAvailable": {},


Looking at these strings again
https://github.com/kubernetes/kubernetes/blob/master/pkg/controller/deployment/util/deployment_util.go#L67

Can we refer to these here instead and create a complete list based on what's available in deployment_util.go?

Suggested change

var (

allowedDeploymentReasons = map[string]struct{}{

"MinimumReplicasAvailable": {},

import deploymentutil "k8s.io/kubernetes/pkg/controller/deployment/util"

var (

allowedDeploymentReasons = map[string]struct{}{

deploymentutil.MinimumReplicasAvailable: {},

@mrueg, When I'm trying to:

go get k8s.io/kubernetes/pkg/controller/deployment/util

I'm getting this:

k8s.io/controller-manager/pkg/features: reading k8s.io/controller-manager/go.mod at revision v0.0.0: unknown revision v0.0.0

Is this a good idea to use internal k8s packages directly? should I directly use strings?

Good point, you're right!

Can you copy all other reasons over as a string and add a comment that they are taken from deployment_utils.go?

@mrueg thanks for the review, made the changes

CatherineF-dev · 2025-08-13T17:54:31Z

/lgtm
/hold
@mrueg for the final review

k8s-ci-robot · 2025-08-13T17:54:39Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: CatherineF-dev, Rishab87

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [CatherineF-dev]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

CatherineF-dev · 2025-08-13T17:54:50Z

/triage accepted

mrueg · 2025-08-13T18:30:48Z

internal/store/deployment.go

+		"DeploymentPaused":           {},
+		"DeploymentResumed":          {},
+		"MinimumReplicasAvailable":   {},
+		"MinimumReplicasUnavailable": {},


This is missing the "" if it's unavailable.

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Aug 4, 2025

k8s-ci-robot requested review from logicalhan and rexagod August 4, 2025 16:34

k8s-ci-robot added needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Aug 4, 2025

added reason label

0a50cab

Rishab87 force-pushed the reason-label branch from d37d5cb to 0a50cab Compare August 5, 2025 11:11

mrueg reviewed Aug 6, 2025

View reviewed changes

Rishab87 mentioned this pull request Aug 7, 2025

[LFX Mentorship] Automate the release process #2711

Open

k8s-ci-robot assigned mrueg Aug 7, 2025

k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Aug 7, 2025

changing empty reasons to unkown

b30335f

mrueg reviewed Aug 11, 2025

View reviewed changes

fixing empty reasons and adding tests

bc1e7f8

mrueg reviewed Aug 11, 2025

View reviewed changes

adding empty reason in allowedList

b582038

mrueg reviewed Aug 12, 2025

View reviewed changes

adding reasons from deployment_utils.go

547afad

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 13, 2025

k8s-ci-robot assigned CatherineF-dev Aug 13, 2025

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 13, 2025

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 13, 2025

mrueg reviewed Aug 13, 2025

View reviewed changes

feat: include reason label to kube_deployment_status_condition #2719

Are you sure you want to change the base?

feat: include reason label to kube_deployment_status_condition #2719

Conversation

Rishab87 commented Aug 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dgrisonnet commented Aug 7, 2025

Uh oh!

Rishab87 commented Aug 11, 2025

Uh oh!

mrueg Aug 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Rishab87 commented Aug 11, 2025

Uh oh!

mrueg Aug 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Rishab87 Aug 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Rishab87 commented Aug 12, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Rishab87 Aug 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

CatherineF-dev commented Aug 13, 2025

Uh oh!

k8s-ci-robot commented Aug 13, 2025

Uh oh!

CatherineF-dev commented Aug 13, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

feat: include reason label to `kube_deployment_status_condition` #2719

feat: include reason label to `kube_deployment_status_condition` #2719

Rishab87 commented Aug 4, 2025 •

edited

Loading

mrueg Aug 11, 2025 •

edited

Loading

mrueg Aug 11, 2025 •

edited

Loading

Rishab87 Aug 11, 2025 •

edited

Loading

Rishab87 Aug 13, 2025 •

edited

Loading